Generate a random Drip List ID when creating a new Drip List #1406
Conversation
|
🚅 Previously deployed to Railway in the Drips App project. Environment has been deleted. |
mhgbrown
force-pushed
the
mhgbrown-991-generate-a-random-drip-list-id-when-creating-a-new-drip-list
branch
from
5f5c3f2 to
0f3ed61
Compare
| // We use the count of *all* NFT sub-accounts to generate the salt for the Drip List ID. | ||
| // This is because we want to avoid making HTTP requests to the subgraph for each NFT sub-account to check if it's a Drip List. | ||
| private _calcSaltFromAddress = (address: string, listCount: number): bigint /* 64bit */ => { | ||
| // Create random salt from address |
There was a problem hiding this comment.
Are we sure this generates a random number?
I think for the same address it will produce the same output, no?
(Even the previous code, I believe wasn't truly random.)
There was a problem hiding this comment.
We're not sure and it looks like it doesn't based on the error I'm receiving. Will look into it!
* move calculateSaltFromAddress to util function
| ); | ||
| const addressBigInt = ethers.toBigInt('0x' + hash.slice(26)); | ||
|
|
||
| const randomBytes = ethers.randomBytes(32); |
There was a problem hiding this comment.
This definitely introduces randomness now 👍
But, why not simplifying to something simple like:
export default function generateRandomNumber() {
const randomBytes = ethers.randomBytes(32);
return BigInt(ethers.hexlify(randomBytes)) & BigInt('0xFFFFFFFFFFFFFFFF');
}This is purely random compared to what we had before where the final result is partially influenced by the (deterministic) seed and address.
There was a problem hiding this comment.
No reason at all. Just not trying to shake the tree too much because I don't know what constraints the seed should have. Will update!
| @@ -108,7 +108,7 @@ export default class DripListService { | |||
| items, | |||
| ); | |||
|
|
|||
| const salt = this._calcSaltFromAddress(this._ownerAddress); | |||
| const salt = this._calcSaltFromAddress(); | |||
There was a problem hiding this comment.
Just a minor suggestion (otherwise feel free to close it 👍): Why not directly calling calculateRandomSalt instead of wrapping it in a "private" _calcSaltFromAddress.
There was a problem hiding this comment.
No real important reason - just trying to keep the integrity of the class's internal API. I'll just use the utility directly
mhgbrown
deleted the
mhgbrown-991-generate-a-random-drip-list-id-when-creating-a-new-drip-list
branch
Removes the count of Drip Lists when determining a salt for a new Drip List.